[PAC][ELF] Place AUTH constants in .data.rel.ro for position dependent code #150418

kovdan01 · 2025-07-24T13:32:08Z

For AArch64 target machine, effective relocation model on Windows and Darwin is always PIC, while for ELF targets Static is used when DynamicNoPIC is requested (see getEffectiveRelocModel in AArch64TargetMachine.cpp).

This resulted in using .rodata section for AUTH constants, which is wrong since these are filled with AUTH dynamic relocs and require the section to be writeable during dynamic relocation resolving.

This patch adds a check ensuring if the constant itself or one of the nested constants are AUTH ones. If so, use .data.rel.ro section.

kovdan01 · 2025-07-24T13:33:25Z

This fixes the issue reported by @pcc in pauth channel in Discord: https://discord.com/channels/636084430946959380/1133112394701348895/1395957067847700630

llvmbot · 2025-07-24T13:34:34Z

@llvm/pr-subscribers-backend-aarch64

Author: Daniil Kovalev (kovdan01)

Changes

For AArch64 target machine, effective relocation model on Windows and Darwin is always PIC, while for ELF targets Static is used when DynamicNoPIC is requested (see getEffectiveRelocModel in AArch64TargetMachine.cpp).

This resulted in using .rodata section for AUTH constants, which is wrong since these are filled with AUTH dynamic relocs and require the section to be writeable during dynamic relocation resolving.

This patch adds a check ensuring if the constant itself or one of the nested constants are AUTH ones. If so, use .data.rel.ro section.

Full diff: https://github.com/llvm/llvm-project/pull/150418.diff

2 Files Affected:

(modified) llvm/lib/Target/TargetLoweringObjectFile.cpp (+17-2)
(added) llvm/test/CodeGen/AArch64/ptrauth-global-no-pic.ll (+51)

diff --git a/llvm/lib/Target/TargetLoweringObjectFile.cpp b/llvm/lib/Target/TargetLoweringObjectFile.cpp
index 9b03e85ca45bf..246509b28cd9c 100644
--- a/llvm/lib/Target/TargetLoweringObjectFile.cpp
+++ b/llvm/lib/Target/TargetLoweringObjectFile.cpp
@@ -17,6 +17,7 @@
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
+#include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/Mangler.h"
 #include "llvm/IR/Module.h"
@@ -220,6 +221,20 @@ void TargetLoweringObjectFile::emitPseudoProbeDescMetadata(MCStreamer &Streamer,
   }
 }
 
+static bool containsConstantPtrAuth(const Constant *C) {
+  if (isa<ConstantPtrAuth>(C))
+    return true;
+
+  if (isa<BlockAddress>(C) || isa<GlobalValue>(C))
+    return false;
+
+  for (const Value *Op : C->operands())
+    if (containsConstantPtrAuth(cast<Constant>(Op)))
+      return true;
+
+  return false;
+}
+
 /// getKindForGlobal - This is a top-level target-independent classifier for
 /// a global object.  Given a global variable and information from the TM, this
 /// function classifies the global in a target independent manner. This function
@@ -327,9 +342,9 @@ SectionKind TargetLoweringObjectFile::getKindForGlobal(const GlobalObject *GO,
       // mergable section, because the linker doesn't take relocations into
       // consideration when it tries to merge entries in the section.
       Reloc::Model ReloModel = TM.getRelocationModel();
-      if (ReloModel == Reloc::Static || ReloModel == Reloc::ROPI ||
+      if ((ReloModel == Reloc::Static || ReloModel == Reloc::ROPI ||
           ReloModel == Reloc::RWPI || ReloModel == Reloc::ROPI_RWPI ||
-          !C->needsDynamicRelocation())
+          !C->needsDynamicRelocation()) && !containsConstantPtrAuth(C))
         return SectionKind::getReadOnly();
 
       // Otherwise, the dynamic linker needs to fix it up, put it in the
diff --git a/llvm/test/CodeGen/AArch64/ptrauth-global-no-pic.ll b/llvm/test/CodeGen/AArch64/ptrauth-global-no-pic.ll
new file mode 100644
index 0000000000000..651f384ef6497
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/ptrauth-global-no-pic.ll
@@ -0,0 +1,51 @@
+; RUN: llc -mtriple aarch64-elf --relocation-model=static         -mattr=+pauth -filetype=asm -o - %s | FileCheck %s
+; RUN: llc -mtriple aarch64-elf --relocation-model=dynamic-no-pic -mattr=+pauth -filetype=asm -o - %s | FileCheck %s
+
+;; A constant value, use .rodata
+; CHECK:         .section        .rodata,"a",@progbits
+; CHECK:         .globl  Const
+; CHECK: Const:
+; CHECK:         .xword  37
+
+;; An AUTH reloc is needed, use .data.rel.ro
+; CHECK:         .section        .data.rel.ro,"aw",@progbits
+; CHECK:         .globl  PtrAuthExtern
+; CHECK: PtrAuthExtern:
+; CHECK:         .xword  ConstExtern@AUTH(da,0)
+
+; CHECK:         .globl  PtrAuth
+; CHECK: PtrAuth:
+; CHECK:         .xword  Const@AUTH(da,0)
+
+; CHECK:         .globl  PtrAuthExternNested1
+; CHECK: PtrAuthExternNested1:
+; CHECK:         .xword  ConstExtern@AUTH(da,0)
+
+;; The address could be filled statically, use .rodata
+; CHECK:         .section        .rodata,"a",@progbits
+; CHECK:         .globl  PtrAuthExternNested2
+; CHECK: PtrAuthExternNested2:
+; CHECK:         .xword  PtrAuthExtern
+
+;; An AUTH reloc is needed, use .data.rel.ro
+; CHECK:         .section        .data.rel.ro,"aw",@progbits
+; CHECK:         .globl  PtrAuthNested1
+; CHECK: PtrAuthNested1:
+; CHECK:         .xword  Const@AUTH(da,0)
+
+;; The address could be filled statically, use .rodata
+; CHECK:         .section        .rodata,"a",@progbits
+; CHECK:         .globl  PtrAuthNested2
+; CHECK: PtrAuthNested2:
+; CHECK:         .xword  PtrAuth
+
+@ConstExtern = external global i64
+@Const       = constant i64 37
+
+@PtrAuthExtern = constant ptr ptrauth (ptr @ConstExtern, i32 2)
+@PtrAuth       = constant ptr ptrauth (ptr @Const,  i32 2)
+
+@PtrAuthExternNested1 = constant { ptr } { ptr ptrauth (ptr @ConstExtern, i32 2) }
+@PtrAuthExternNested2 = constant { ptr } { ptr @PtrAuthExtern }
+@PtrAuthNested1       = constant { ptr } { ptr ptrauth (ptr @Const, i32 2) }
+@PtrAuthNested2       = constant { ptr } { ptr @PtrAuth }

github-actions · 2025-07-24T13:35:32Z

✅ With the latest revision this PR passed the C/C++ code formatter.

…t code For AArch64 target machine, effective relocation model on Windows and Darwin is always PIC, while for ELF targets Static is used when DynamicNoPIC is requested (see `getEffectiveRelocModel` in AArch64TargetMachine.cpp). This resulted in using .rodata section for AUTH constants, which is wrong since these are filled with AUTH dynamic relocs and require the section to be writeable during dynamic relocation resolving. This patch adds a check ensuring if the constant itself or one of the nested constants are AUTH ones. If so, use .data.rel.ro section.

atrosinenko · 2025-07-30T16:24:14Z

llvm/test/CodeGen/AArch64/ptrauth-global-no-pic.ll

+; RUN: llc -mtriple aarch64-elf --relocation-model=static         -mattr=+pauth -filetype=asm -o - %s | FileCheck %s
+; RUN: llc -mtriple aarch64-elf --relocation-model=dynamic-no-pic -mattr=+pauth -filetype=asm -o - %s | FileCheck %s


As far as I can see, -filetype=asm is the default in llc. Furthermore, it is advised to pass the input to opt via redirection - not sure if it has any relation to llc (except for uniformity), but using < %s makes -o - useless, too.

Thanks for suggestion! Fixed in d85f8cf. Sorry for a delay

kovdan01 · 2025-09-05T07:56:28Z

Would be glad to see everyone's feedback on the changes.

kovdan01 · 2025-09-15T08:12:27Z

Would be glad to see everyone's feedback on the changes.

kovdan01 · 2025-09-22T15:12:15Z

Would be glad to see everyone's feedback on the changes.

kovdan01 · 2025-09-28T21:29:16Z

Would be glad to see everyone's feedback on the changes.

kovdan01 · 2025-09-28T21:31:00Z

This fixes the issue reported by @pcc in pauth channel in Discord: https://discord.com/channels/636084430946959380/1133112394701348895/1395957067847700630

@pcc Could you please take a look at this and check if this fixes the issue for you (since this PR was inspired by your issue report)?

kovdan01 · 2025-10-06T13:46:13Z

This fixes the issue reported by @pcc in pauth channel in Discord: https://discord.com/channels/636084430946959380/1133112394701348895/1395957067847700630

@pcc Could you please take a look at this and check if this fixes the issue for you (since this PR was inspired by your issue report)?

Everyone else interested is also welcome to leave feedback :)

atrosinenko · 2025-10-06T14:39:34Z

llvm/lib/Target/TargetLoweringObjectFile.cpp

+  if (isa<BlockAddress>(C) || isa<GlobalValue>(C))
+    return false;


IIUC this would make containsConstantPtrAuth return false for global variable like this

@global_var = global ptr ptrauth (ptr @f, i32 0)

which is still "constant" (as a constant pointer to global_var). This doesn't seem to hurt in case of this PR, but possibly a bit unexpected w.r.t. function name vs. its behavior.

On the other hand, I would expect containsConstantPtrAuth to return false for @ptr_to_global_var = constant ptr @global_var no matter how @global_value is defined - I guess recursively inspecting GlobalValues would make containsConstantPtrAuth incorrectly return true in such cases. Thus, the original "issue" should be considered as a matter of comments or naming, if at all.

atrosinenko · 2025-10-06T15:10:22Z

llvm/lib/Target/TargetLoweringObjectFile.cpp

-      if (ReloModel == Reloc::Static || ReloModel == Reloc::ROPI ||
-          ReloModel == Reloc::RWPI || ReloModel == Reloc::ROPI_RWPI ||
-          !C->needsDynamicRelocation())
+      if ((ReloModel == Reloc::Static || ReloModel == Reloc::ROPI ||
+           ReloModel == Reloc::RWPI || ReloModel == Reloc::ROPI_RWPI ||
+           !C->needsDynamicRelocation()) &&
+          !containsConstantPtrAuth(C))
        return SectionKind::getReadOnly();


[nit] Maybe it would be more readable to organize the checks along the lines

// ... } else { // First of all, the dynamic linker always needs to fix PtrAuth relocations up. if (containsPtrAuth(C)) return SectionKind::getReadOnlyWithRel(); // In static, ROPI and RWPI relocation models, the linker will resolve // ... if (ReloModel == ...) return SectionKind::getReadOnly(); // Otherwise, the dynamic linker needs to fix it up, put it in the // writable data.rel section. return SectionKind::getReadOnlyWithRel();

I've always found it a bit confusing that Constant::needsDynamicRelocation really means "needs dynamic relocation in non-static/R*PI relocation models". Would it make sense to move the check for relocation model as well as the ptrauth constant detection (without the reloc model check of course) into needsDynamicRelocation? Then this could end up being:

if (!C->needsDynamicRelocation(TM.getRelocationModel())) return SectionKind::getReadOnly();

[nit] Maybe it would be more readable to organize the checks along the lines

@atrosinenko Applied your suggestion in cafc9df, thanks!

Would it make sense to move the check for relocation model as well as the ptrauth constant detection (without the reloc model check of course) into needsDynamicRelocation?

@pcc While I like the idea of simplifying things and making naming less confusing, I'm not sure if such a change would make logic for all the users of needsDynamicRelocation simpler. Also, it looks like that some callers do not have easy access to TargetMachine object, so they can't easily call getRelocationModel() and pass that to new needsDynamicRelocation updated accordingly to your suggestion.

We might think of such an improvement, but I guess this is out of scope of this PR and it's better not to combine actual bug fixes and refactoring.

Can we postpone applying your suggestion? And could you please tell if there is any other feedback on this PR or this can be merged? Thanks!

kovdan01 added this to Pointer Authentication Tasks Jul 24, 2025

kovdan01 moved this to In Progress in Pointer Authentication Tasks Jul 24, 2025

kovdan01 self-assigned this Jul 24, 2025

kovdan01 requested review from HaohaiWen, MaskRay, asl, atrosinenko and pcc July 24, 2025 13:33

kovdan01 marked this pull request as ready for review July 24, 2025 13:33

llvmbot added the backend:AArch64 label Jul 24, 2025

kovdan01 force-pushed the pauth-data-rel-ro branch from f6170f2 to a91b0ee Compare July 24, 2025 13:39

atrosinenko reviewed Jul 30, 2025

View reviewed changes

kovdan01 added 2 commits September 5, 2025 10:44

Merge branch 'main' into pauth-data-rel-ro

bf6186d

Address review comments

d85f8cf

kovdan01 requested a review from atrosinenko September 5, 2025 07:56

atrosinenko reviewed Oct 6, 2025

View reviewed changes

kovdan01 added 2 commits October 19, 2025 21:42

Merge branch 'main' into pauth-data-rel-ro

1868cbe

Apply Anatoly's suggestion

cafc9df

kovdan01 force-pushed the pauth-data-rel-ro branch from 4097da3 to cafc9df Compare October 19, 2025 19:26

kovdan01 requested a review from atrosinenko October 19, 2025 21:57

		; RUN: llc -mtriple aarch64-elf --relocation-model=static -mattr=+pauth -filetype=asm -o - %s \| FileCheck %s
		; RUN: llc -mtriple aarch64-elf --relocation-model=dynamic-no-pic -mattr=+pauth -filetype=asm -o - %s \| FileCheck %s

		if (isa<BlockAddress>(C) \|\| isa<GlobalValue>(C))
		return false;

[PAC][ELF] Place AUTH constants in .data.rel.ro for position dependent code #150418

Are you sure you want to change the base?

[PAC][ELF] Place AUTH constants in .data.rel.ro for position dependent code #150418

Uh oh!

Conversation

kovdan01 commented Jul 24, 2025

Uh oh!

kovdan01 commented Jul 24, 2025

Uh oh!

llvmbot commented Jul 24, 2025

Uh oh!

github-actions bot commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atrosinenko Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

kovdan01 Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kovdan01 commented Sep 5, 2025

Uh oh!

kovdan01 commented Sep 15, 2025

Uh oh!

kovdan01 commented Sep 22, 2025

Uh oh!

kovdan01 commented Sep 28, 2025

Uh oh!

kovdan01 commented Sep 28, 2025

Uh oh!

kovdan01 commented Oct 6, 2025

Uh oh!

atrosinenko Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

atrosinenko Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atrosinenko Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

pcc Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

kovdan01 Oct 19, 2025

Choose a reason for hiding this comment

Uh oh!

kovdan01 Oct 19, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Jul 24, 2025 •

edited

Loading

kovdan01 Sep 5, 2025 •

edited

Loading

atrosinenko Oct 6, 2025 •

edited

Loading